Open Bug 712497 Opened 13 years ago Updated 3 years ago

Clang Static Analysis: Dereference of undefined pointer value in nsprpub/pr/src/misc/prtpool.c

Categories

(NSPR :: NSPR, defect)

defect

Tracking

(Not tracked)

People

(Reporter: decoder, Unassigned)

References

(Blocks 1 open bug, )

Details

The following report (in the URL field) has been generated by static analysis using Clang. It would be good if someone familiar with the particular code could check if - this is really a bug or a false positive - and/or if it makes sense to adjust the code (even if there is not a real bug present, e.g. by adding a missing initialization). In this particular report, I think the problem is that | pollfds[pollfds_used] | could be accessed without taking the | if(..) | branch before that allocates this array. Even if this could never happen (which I don't know), it would probably be a good idea to initialize the | pollfds | pointer to NULL, so it would at most be a null pointer deference.
Assignee: nobody → wtc
Component: General → NSPR
Product: Core → NSPR
QA Contact: general → nspr
Version: Trunk → 4.9
Thanks for the bug report. This is a false positive. The PRThreadPool structure pointed to by 'tp' is allocated by alloc_threadpool(). The structure is allocated with a PR_CALLOC call, so every member is initialized to 0, including tp->ioq.cnt and tp->ioq.npollfds. So the first time we enter the while loop in io_wstart, the code pollfd_cnt = tp->ioq.cnt + 10; if (pollfd_cnt > tp->ioq.npollfds) { evaluates to true, and so we will do pollfds = tp->ioq.pollfds; Since this requires looking at three functions (alloc_threadpool, PR_CreateThread, and io_wstart), it may be beyond the ability of static analyzers. If we initialize pollfds to NULL, will that fix this Clang report? PRPollDesc *pollfds = NULL;
(In reply to Wan-Teh Chang from comment #1) > Thanks for the bug report. This is a false positive. > Thanks for investigating! > If we initialize pollfds to NULL, will that fix this Clang report? > > PRPollDesc *pollfds = NULL; It would turn the message from "Dereference of undefined pointer" to "Possible null dereference". That would still help us though because we usually ignore the null dereferences (because they're not security critical in our context) and we can also annotate the code for that (I think, haven't looked into that yet). So yes, changing that seems helpful although Clang will throw a different message then.

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Assignee: wtc → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.